Skip to content

Make HTTP/2 response body close idempotent - #26

Merged
bogdanfinn merged 1 commit into
bogdanfinn:masterfrom
kernel:hypeship/idempotent-response-close-upstream
Sep 4, 2026
Merged

Make HTTP/2 response body close idempotent#26
bogdanfinn merged 1 commit into
bogdanfinn:masterfrom
kernel:hypeship/idempotent-response-close-upstream

Conversation

@rgarcia

@rgarcia rgarcia commented Sep 2, 2026

Copy link
Copy Markdown

transportResponseBody.Close currently performs its reset logic again when called more than once. For an unfinished response, two calls emit two RST_STREAM(CANCEL) frames. After a response body has already been read to EOF, the first Close records the closed-body error and a second Close incorrectly emits a reset.

Treat a body already closed by the caller as a successful no-op. This preserves the first close's existing behavior while preventing repeated reset and flow-control side effects.

The regression test covers both cases:

  • an unfinished response body emits exactly one RST_STREAM(CANCEL) across two Close calls
  • a response body read to EOF emits no RST_STREAM across two Close calls

Test results

Passed on Go 1.24.1 (toolchain selected by the current upstream go.mod):

go test ./http2 -run '^TestTransportResponseBodyCloseIsIdempotent$' -count=1
ok  github.com/bogdanfinn/fhttp/http2  1.019s

go test -race ./http2 -run '^TestTransportResponseBodyCloseIsIdempotent$' -count=1
ok  github.com/bogdanfinn/fhttp/http2  2.043s

go test ./http2 -run '^TestTransportResponseBodyCloseIsIdempotent$' -count=10
ok  github.com/bogdanfinn/fhttp/http2  10.060s

The regression test fails against unpatched upstream as expected:

unfinished: received 2 RST_STREAM frames; want 1
after EOF: received 1 RST_STREAM frames; want 0

The full package command was also run:

go test ./http2 -count=1

It completed in 94.007s with one existing failure in TestTransportRejectsConnHeaders (Content-Length unexpectedly present). The same failure reproduces on a clean checkout of the upstream base commit, so it is unrelated to this change.

Treat a response body already closed by the caller as a no-op so repeated Close calls do not emit additional RST_STREAM frames.
@bogdanfinn
bogdanfinn merged commit 82b70d5 into bogdanfinn:master Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants